[문자열 계산기] 김경수 제출 (1주차) by Siul49 · Pull Request #9 · Java-JavaScript-Language-Stuty/java-calculator-7

Siul49 · 2025-02-07T14:41:12Z

gradlew.bat이 안된다고 뜨네요.

java 버전 확인하려고 해도

이렇게 뜨고 안떠서 제대로 작동하는지 확인도 못해보고 제출하네요...

이번 주차는 넘어야 할 벽의 높이를 알게되는 값진 주차였던 것 같습니다 :)
피드백 해주시면 감사하겠습니다.

moongua404

1주차 미션 수고많으셨습니다!

moongua404 · 2025-02-09T05:02:15Z

+                throw new IllegalArgumentException("잘못된 문자가 있는 것 같은데요?");
+            }
+        }
+    }


함수 깊이가 너무 깊은 것 같아요! 더 분리하면 좋을 것 같아요
특히 if elseif가 많이 섞여있어서 가독성이 떨어지는 것 같아요

정규표현식 관련 리뷰와 합쳐서 동의하는게 정규표현식으로 커스텀 구분자를 추출한다면 코드가 확실히 짧아질 수 있을 것 같아요!

moongua404 · 2025-02-09T05:08:38Z

+                else {
+                    throw new IllegalArgumentException("잘못된 문자가 있는 것 같은데요?");
+                }
+            }


이 윗부분에서 구분자 추출하는 부분은 정규표현식으로 처리하고 밑 부분은 인덱스 접근이 아닌 원소 순회로 표현하면 가독성이 더 높을 것 같아요

정규표현식 기억하겠습니다..! Matcher와 Pattern 말씀하시는거죠?
인덱스 접근 부분은 생각해보니 원소 순회가 가독성이 더 좋을 것 같네요 감사합니다!

moongua404 · 2025-02-09T05:10:48Z

+        // 문자열 입력받기
+        System.out.println("덧셈할 문자열을 입력해 주세요.");
+        Input();
+        for (int i = 0; i < userInput.length(); i++){


Character을 순회하면 일의자리를 제외한 수를 표현할 수 없지 않나요?
ex) 32,1 -> 3+2+1 -> 6

저도 그렇게 생각해서 아래 if문 부분이 해결된다면 숫자 추출을 문자열 내부 함수로 처리할 수 있어 보입니다.

moongua404 · 2025-02-09T05:12:55Z

+        a.extractNum();
+        a.sumNumbers(a.getNumbers());
    }
 }


전체적으로 역할을 분리하지 않아서 코드 흐름이 흔들리는게 있는 것 같아요! 기능을 쪼개고 기능마다 메서드를 만들어 구현한다면 좀 더 쉽게 표현될 수 있을 것 같아요!

Regyung

1주차 수고하셨습니다 남은 주차도 같이 열심히 해봅시다!

Regyung · 2025-02-09T13:40:05Z

+        // 문자열 입력받기
+        System.out.println("덧셈할 문자열을 입력해 주세요.");
+        Input();
+        for (int i = 0; i < userInput.length(); i++){


저도 그렇게 생각해서 아래 if문 부분이 해결된다면 숫자 추출을 문자열 내부 함수로 처리할 수 있어 보입니다.

Regyung · 2025-02-09T13:40:08Z

+                // 개행문자를 문자열로 넣는 과정을 추가하기보단
+                // 그냥 다시 문자열을 받아도 상관 없겠다는 생각을 했어요
+                // 근데 구분자를 추가하고 끝나는 경우도 있다는 생각에
+                // if 문을 하나 더 넣었네요


코드 외의 이야기지만 긴 문장의 주석을 작성한다면 각 줄마다 '//'로 주석을 다는 것보다 '/* */'과 같은 블록주석 표현을 쓰는 것도 좋다고 생각합니다.

좋은 말씀 감사합니다 ㅎㅎ

Regyung · 2025-02-09T13:40:11Z

+                throw new IllegalArgumentException("잘못된 문자가 있는 것 같은데요?");
+            }
+        }
+    }


정규표현식 관련 리뷰와 합쳐서 동의하는게 정규표현식으로 커스텀 구분자를 추출한다면 코드가 확실히 짧아질 수 있을 것 같아요!

Regyung · 2025-02-09T13:41:17Z

+        System.out.println("덧셈할 문자열을 입력해 주세요.");
+        Input();
+        for (int i = 0; i < userInput.length(); i++){
+            if ((userInput.charAt(i) == '/' && userInput.charAt(i+1) == '/')) {


for문의 수행 횟수가 문자열의 길이와 일치하기 때문에 'i+1'과 같은 접근 표현을 사용 시 인덱스가 오버될 수도 있어보입니다.

아 여기 인덱스 신경을 못썼네요. 이런 곳에서 for문보다는 stream을 사용하는게 더 괜찮을까요?

1jae2jae3jae · 2025-02-10T05:36:14Z

+                    }
+                }
+                else {
+                    throw new IllegalArgumentException("잘못된 문자가 있는 것 같은데요?");


예외가 생길 조건을 따로 지정해도 좋을 것 같아요!

1jae2jae3jae · 2025-02-10T05:38:50Z

+        a.extractNum();
+        a.sumNumbers(a.getNumbers());
    }
 }


기능마다 나눠서 코드를 짜면 좋은 코드가 될 수 있을 것 같아요! 저랑 비슷하지만 다르게 접근하셔서 대단한 것 같습니다!

1jae2jae3jae · 2025-02-10T06:07:18Z

+                // if 문을 하나 더 넣었네요
+                if (i+2 < userInput.length()) {
+                    separator.add(String.valueOf(userInput.charAt(i + 2)));
+                    if(userInput.length() > i + 4) {


여기서는 커스텀 구분자 이후에 나오는 input을 받기 위해서 i+4로 한 건가요?

//"커스텀구분자"\n로 커스텀 구분자가 추가되기만 하고 더이상 입력이 없을 경우에 Input()을 받게 된다면 오류가 발생하기 때문에 방지하기 위해 i+4조건을 붙여주었습니다

1jae2jae3jae · 2025-02-10T06:19:51Z

+    private ArrayList<String> separator;
+
+    public Application(){
+        numbers = new ArrayList<String>();


Application()에 안에 numbers와 separator을 선언한 이유가 따로 있나요?

MVC를 아직 공부하지 못해 모든 기능을 Application() 안에 다 넣어서 그렇습니다 ㅠㅜ

Hey0140 · 2025-02-11T10:03:38Z

+    public void Input(){
+        userInput = Console.readLine();
+    }
+


함수들로 깔끔하게 정리해준 거 가독성 좋아보여서 좋은 것 같습니다!

Siul added 3 commits February 7, 2025 23:18

readme 작성

acefd10

거의 다 한듯?

44bb412

더이상 모르겠어

25ffea8

Siul49 changed the title ~~1주차 미션~~ [문자열 계산기] 김경수 제출 (1주차) Feb 7, 2025

moongua404 reviewed Feb 9, 2025

View reviewed changes

Regyung reviewed Feb 9, 2025

View reviewed changes

1jae2jae3jae reviewed Feb 10, 2025

View reviewed changes

Hey0140 reviewed Feb 11, 2025

View reviewed changes

Uh oh!

Conversation

Siul49 commented Feb 7, 2025

Uh oh!

moongua404 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Regyung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants